monai/apps/reconstruction/transforms/dictionary.py (2)

47-51: Missing Raises section in docstring.

The __call__ method raises KeyError when a requested key is absent and allow_missing_keys=False. Per coding guidelines, docstrings should document raised exceptions.

📝 Proposed docstring addition

         allow_missing_keys: don't raise exception if key is missing
+
+    Raises:
+        KeyError: If ``meta_key`` is not found in the data dictionary, or if a
+            requested key is missing from the metadata and ``allow_missing_keys``
+            is False.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@monai/apps/reconstruction/transforms/dictionary.py` around lines 47 - 51, The
docstring for the transform is missing a Raises section documenting that
__call__ can raise KeyError when a requested key is absent and
allow_missing_keys is False; update the docstring for the class or the __call__
method (reference symbols: __call__, keys, meta_key, allow_missing_keys) to add
a "Raises" section that clearly states KeyError is raised in that situation and
any conditions under which it is not raised.

---

83-90: Consider validating meta_obj type.

If meta_key references a value that is neither MetaTensor nor dict, the code will fail at line 93 with an unclear TypeError. Adding a type check would provide a clearer error message.

🛡️ Proposed defensive check

         if isinstance(meta_obj, MetaTensor):
             meta_dict = meta_obj.meta
-        else:
+        elif isinstance(meta_obj, dict):
             meta_dict = meta_obj
+        else:
+            raise TypeError(
+                f"meta_key `{self.meta_key}` must reference a MetaTensor or dict, "
+                f"got {type(meta_obj).__name__}"
+            )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@monai/apps/reconstruction/transforms/dictionary.py` around lines 83 - 90,
meta_obj retrieved by meta_key may be neither a MetaTensor nor a dict, so add an
explicit type check after obtaining meta_obj (in the block that currently tests
isinstance(meta_obj, MetaTensor)) to validate allowed types: if meta_obj is a
MetaTensor set meta_dict = meta_obj.meta; elif it's a dict set meta_dict =
meta_obj; else raise a clear TypeError referencing meta_key and the actual type
of meta_obj. Update the code paths around meta_key / meta_obj / meta_dict to use
this validated meta_dict afterwards.

tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py (2)

119-122: Consider adding test for missing meta_key.

No test covers the case when meta_key itself is not present in the data dictionary. This would raise KeyError at d[self.meta_key].

🧪 Proposed edge case test

def test_missing_meta_key_raises(self):
    """Test that missing meta_key raises KeyError."""
    data = {"image": torch.zeros(1, 2, 2)}
    transform = ExtractDataKeyFromMetaKeyd(keys="filename_or_obj", meta_key="nonexistent_meta")
    with self.assertRaises(KeyError):
        transform(data)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py`
around lines 119 - 122, Add a test covering the case where the meta_key is
absent: create a test method (e.g., test_missing_meta_key_raises) that
constructs input data without the specified meta_key, instantiates
ExtractDataKeyFromMetaKeyd(keys="filename_or_obj", meta_key="nonexistent_meta"),
and asserts that calling transform(data) raises a KeyError; this validates
behavior when d[self.meta_key] is missing.

---

77-84: Missing test for KeyError with dict source.

test_missing_key_raises only tests with MetaTensor. Add a similar test for dict-based metadata to ensure symmetry.

🧪 Proposed additional test

def test_missing_key_raises_dict(self):
    """Test that a missing key raises KeyError when allow_missing_keys=False with dict."""
    data = {
        "image": torch.zeros(1, 2, 2),
        "image_meta_dict": {"filename_or_obj": "image.nii"},
    }
    transform = ExtractDataKeyFromMetaKeyd(keys="nonexistent_key", meta_key="image_meta_dict")
    with self.assertRaises(KeyError):
        transform(data)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py`
around lines 77 - 84, Add a parallel unit test to cover the dict-based metadata
case: create a test function (e.g., test_missing_key_raises_dict) that
constructs a plain tensor data dict with an accompanying meta dict key (e.g.,
"image_meta_dict") missing the requested meta field, instantiate
ExtractDataKeyFromMetaKeyd(keys="nonexistent_key", meta_key="image_meta_dict"),
and assert it raises KeyError when called; mirror the existing
test_missing_key_raises structure but use a raw torch tensor for "image" and a
dict for the meta source to ensure symmetry between MetaTensor and dict
handling.

tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py (1)

23-105: Docstrings are not in required Google-style sections.

Definitions have docstrings, but they don’t document Args/Returns/Raises in Google-style format.

As per coding guidelines, "Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py`
around lines 23 - 105, The test methods (e.g., test_extract_from_dict,
test_extract_from_metatensor, test_extract_multiple_keys_from_metatensor,
test_missing_key_raises, test_missing_key_allowed_metatensor,
test_missing_key_allowed_dict, test_original_data_preserved_metatensor)
currently use free-form docstrings; update each test docstring to Google-style
sections: add Args: (describe inputs like data, transform), Returns: (if any)
and Raises: (e.g., KeyError for test_missing_key_raises) where appropriate,
following the project's docstring guidelines, ensuring each docstring documents
arguments, expected return/side-effect, and exceptions raised for clarity and
consistency with ExtractDataKeyFromMetaKeyd usage.

tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py (1)

100-110: ⚠️ Potential issue | 🟡 Minor

Use identity assertion for “original object preserved”.

torch.equal(...) checks tensor value equality, not object identity. This test should assert the same MetaTensor instance is preserved.

Proposed fix

-        self.assertTrue(torch.equal(result["image"], mt))
+        self.assertIs(result["image"], mt)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py`
around lines 100 - 110, The test test_original_data_preserved_metatensor
currently uses torch.equal(...) which checks tensor value equality; change the
final assertion to check object identity so the original MetaTensor instance is
preserved: replace the torch.equal-based assertion with an identity assertion
(e.g., use self.assertIs or equivalent) on result["image"] and mt in the test
for ExtractDataKeyFromMetaKeyd (keys="filename_or_obj", meta_key="image") to
ensure the same MetaTensor object is returned.

tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py (1)

23-101: Standardize docstrings to Google-style sections.

Docstrings are present, but they currently omit structured sections (Args, Returns, Raises) required by repo guidance.

As per coding guidelines, "Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py`
around lines 23 - 101, The test docstrings are informal; update each test
function (test_extract_from_dict, test_extract_from_metatensor,
test_extract_multiple_keys_from_metatensor,
test_extract_multiple_keys_from_dict, test_missing_key_raises,
test_missing_key_allowed_metatensor, test_missing_key_allowed_dict,
test_original_data_preserved_metatensor) to use Google-style sections: add an
Args section (even if no params, state None or describe fixtures like data/mt),
a Returns section (None), and a Raises section where applicable (e.g.,
test_missing_key_raises should list KeyError), and mention the relevant symbols
under test (ExtractDataKeyFromMetaKeyd, MetaTensor) in the description so the
docstrings conform to the repo’s Google-style docstring guidelines.

tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py (1)

109-111: ⚠️ Potential issue | 🟡 Minor

Check identity here, not tensor equality.

torch.equal can still pass for a different tensor object. This test is about preserving the original MetaTensor reference.

Patch

         self.assertIn("image", result)
         self.assertIsInstance(result["image"], MetaTensor)
-        self.assertTrue(torch.equal(result["image"], mt))
+        self.assertIs(result["image"], mt)

Does PyTorch `torch.equal` check object identity, or only that two tensors have the same shape and values?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py`
around lines 109 - 111, The test currently uses torch.equal to compare
result["image"] and mt which only checks tensor content, not object identity;
replace that assertion with an identity check (e.g., use unittest's assertIs) to
ensure the MetaTensor instance is preserved by the transform—locate the
assertions around result, image, mt in test_extract_data_key_from_meta_keyd.py
and change the final check to assert the object identity of result["image"]
against mt.

tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py (1)

75-100: Add the dict-path failure case.

The negative path is only asserted through MetaTensor. A matching dict-based test with allow_missing_keys=False would lock in the backward-compatible branch too.

As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/apps/reconstruction/transforms/test_extract_data_key_from_meta_keyd.py`
around lines 75 - 100, Add a dict-path negative test mirroring the MetaTensor
case: create data = {"image": torch.zeros(...), "image_meta_dict":
{"filename_or_obj":"image.nii"}} and assert that
ExtractDataKeyFromMetaKeyd(keys="nonexistent_key", meta_key="image_meta_dict",
allow_missing_keys=False") raises KeyError when called; reference the test
helper/test names and the ExtractDataKeyFromMetaKeyd class so the new test sits
alongside test_missing_key_raises, test_missing_key_allowed_metatensor, and
test_missing_key_allowed_dict.

monai/data/meta_tensor.py (1)

174-184: Expand docstrings for new magic methods to match project docstring rules.

Line 175 and Line 181 use short-form docstrings; please add Google-style Args, Returns, and Raises sections for these new definitions.

Proposed docstring-only refinement

-    def __contains__(self, key):
-        """Allow string-key lookups to check ``.meta``, e.g. ``"filename_or_obj" in meta_tensor``."""
+    def __contains__(self, key):
+        """Check membership in metadata or tensor values.
+
+        Args:
+            key: Metadata key (string) or tensor membership query value.
+
+        Returns:
+            bool: ``True`` when found in ``self.meta`` for string keys, otherwise
+            delegates to tensor membership semantics.
+
+        Raises:
+            TypeError: Propagated from tensor membership checks for unsupported key types.
+        """
         if isinstance(key, str):
             return key in self.meta
         return super().__contains__(key)

-    def __getitem__(self, key):
-        """Allow string-key indexing to access ``.meta``, e.g. ``meta_tensor["filename_or_obj"]``."""
+    def __getitem__(self, key):
+        """Get metadata by string key or tensor item by index.
+
+        Args:
+            key: Metadata key (string) or tensor indexing key.
+
+        Returns:
+            Any: Metadata value for string keys, otherwise tensor indexing result.
+
+        Raises:
+            KeyError: If a string metadata key is missing.
+            IndexError: Propagated from tensor indexing when index is invalid.
+            TypeError: Propagated from tensor indexing for unsupported key types.
+        """
         if isinstance(key, str):
             return self.meta[key]
         return super().__getitem__(key)

As per coding guidelines, "Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@monai/data/meta_tensor.py` around lines 174 - 184, Update the short
docstrings on the __contains__ and __getitem__ methods to Google-style
docstrings: for __contains__, add an Args section describing the key (type str
or other), a Returns section stating it returns bool (True if key found in
self.meta or delegating to super), and a Raises section if any exceptions can
propagate (e.g., TypeError if key unsupported); for __getitem__, add an Args
section describing key, a Returns section describing the returned meta value or
ndarray, and a Raises section noting possible KeyError from self.meta or
exceptions from super().__getitem__; keep the existing behavior and mention
.meta and calls to super().__contains__/super().__getitem__ in the descriptions
so reviewers can locate the logic.